Skip to content

fix(spreadsheet): enable multiple optional loading parameters#3835

Merged
TheMaskedTurtle merged 5 commits intomainfrom
jorism/fix/load-several-optional-loading-parameters
Mar 26, 2026
Merged

fix(spreadsheet): enable multiple optional loading parameters#3835
TheMaskedTurtle merged 5 commits intomainfrom
jorism/fix/load-several-optional-loading-parameters

Conversation

@TheMaskedTurtle
Copy link
Contributor

PR Summary

If we modify several loading options at the same time only one is taken into account for reloading related spreadsheet which is not the expected behavior.

For example, if you load lines and generators spreadsheets, if then we toggle on "inactive limit sets" on lines and "regulated terminals" on generators, we expect both types to be completely reloaded to add these data. But for now, if you do that, only the lines will be reloaded. So, this fix enables to reload both lines and generators in this situation.

Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3e96ba1c-7c17-4a04-847b-bdb6ae4399c4

📥 Commits

Reviewing files that changed from the base of the PR and between 87d5c94 and cce49a4.

📒 Files selected for processing (1)
  • src/components/spreadsheet-view/spreadsheet/spreadsheet-content/hooks/use-optional-loading-parameters-for-equipments.ts

📝 Walkthrough

Walkthrough

A conditional chain in a useEffect hook is refactored from mutually exclusive if / else if blocks to independent if blocks, allowing multiple equipment type state updates to execute simultaneously in a single render cycle instead of stopping at the first matching condition.

Changes

Cohort / File(s) Summary
Equipment Optional Loading Parameters Hook
src/components/spreadsheet-view/spreadsheet/spreadsheet-content/hooks/use-optional-loading-parameters-for-equipments.ts
Changed useEffect conditional logic from if / else if chain to multiple independent if blocks for equipment types (LINE, TWO_WINDINGS_TRANSFORMER, GENERATOR, BUS). Enables simultaneous state updates for multiple equipment types in a single render cycle.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: converting mutually exclusive branches to allow multiple optional loading parameters to be updated simultaneously.
Description check ✅ Passed The description clearly explains the problem (only one spreadsheet reloads when multiple options change) and the solution, directly relating to the code change.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

Copy link
Contributor

@dbraquart dbraquart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code: ok

  • maybe we could refactor to call the 2 setState() a single "batched" time rather than N times

tests: ok

  • the fix also works when we unselect several options

@TheMaskedTurtle
Copy link
Contributor Author

code: ok

  • maybe we could refactor to call the 2 setState() a single "batched" time rather than N times

tests: ok

  • the fix also works when we unselect several options

It's batched by react, so we keep as is to be sure to not introduce regressions

@TheMaskedTurtle TheMaskedTurtle merged commit d63b549 into main Mar 26, 2026
6 checks passed
@TheMaskedTurtle TheMaskedTurtle deleted the jorism/fix/load-several-optional-loading-parameters branch March 26, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants